Skip to content

Comments

LG-10052 Validate Zip Code on "Enter the information on your state‑issued ID" page#8554

Merged
gina-yamada merged 9 commits intomainfrom
gyamada/lg-10052-validate-zip-on-state-id
Jun 9, 2023
Merged

LG-10052 Validate Zip Code on "Enter the information on your state‑issued ID" page#8554
gina-yamada merged 9 commits intomainfrom
gyamada/lg-10052-validate-zip-on-state-id

Conversation

@gina-yamada
Copy link
Contributor

@gina-yamada gina-yamada commented Jun 8, 2023

🎫 Ticket

LG-10052 Validate Zip Code on "Enter the information on your state‑issued ID" page

🛠 Summary of changes

  1. Removed maxlength to let regex handle length
  2. Add regex pattern & error message (same pattern, same error as zip on residential address- see screenshot below 👇 )
  3. Add as: tel for mobile numeric key (so it would behave the same as zip code on residential address- see screenshot below 👇 )

📜 Testing Plan

  • locally set in_person_capture_secondary_id_enabled to true
  • Create an account. Fail proofing to hit In Person Proofing Flow
  • Once on "Enter the information on your state‑issued ID" page" enter both valid and non-valid inputs
  • Observe validation/error message when validation failes
  • Confirm behave and error message is consist with residential address by selecting that you live at a different address

👀 Screenshots

Before: No validation (length and characters)

Screenshot 2023-06-08 at 10 02 44 AM

After:

Has validation + error message

Passes Validation

Screenshot 2023-06-08 at 9 41 11 AM

Screenshot 2023-06-08 at 9 40 38 AM

Fails Validation

Screenshot 2023-06-08 at 9 42 57 AM

Screenshot 2023-06-08 at 9 40 47 AM

Screenshot 2023-06-08 at 10 04 31 AM

For Quick Reference here is a screenshot of app/views/idv/in_person/address.html.erb so you can see attributes for component

Screenshot 2023-06-08 at 9 43 19 AM

Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a regression spec for this?

This seems like a good candidate to extract to a common component (e.g. ZipCodeFieldComponent) to avoid future such inconsistency. Another thing I notice is this field probably doesn't have the Cleave formatting that other zipcode fields have, currently via formatted-fields.ts + the .zipcode class.

aduth

This comment was marked as duplicate.

aduth

This comment was marked as duplicate.

@tomas-nava
Copy link
Contributor

Another thing I notice is this field probably doesn't have the Cleave formatting that other zipcode fields have, currently via formatted-fields.ts + the .zipcode class.

Added in f1ccf50

@tomas-nava
Copy link
Contributor

This seems like a good candidate to extract to a common component (e.g. ZipCodeFieldComponent) to avoid future such inconsistency.

I created LG-10056 for this.

@tomas-nava
Copy link
Contributor

Can we add a regression spec for this?

added in 4a57f23

@gina-yamada
Copy link
Contributor Author

gina-yamada commented Jun 9, 2023

@aduth Thanks for the good suggestions yesterday. A few changes pushed up since you last reviewed. Now that the pipeline is fixed can I get a quick review and a thumbs up before merging? Thanks

Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Comment on lines 9 to 10
let(:remote_identity_proofing) { false }

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this used for anything?

Suggested change
let(:remote_identity_proofing) { false }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed with commit ba04a56

@@ -0,0 +1,47 @@
require 'rails_helper'
require 'axe-rspec'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed?

Suggested change
require 'axe-rspec'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed with commit ba04a56

RSpec.describe 'doc auth IPP state ID step', js: true do
include IdvStepHelper
include InPersonHelper
include VerifyStepHelper
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed?

Suggested change
include VerifyStepHelper

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed with commit ba04a56

@gina-yamada gina-yamada merged commit 97cb3e4 into main Jun 9, 2023
@gina-yamada gina-yamada deleted the gyamada/lg-10052-validate-zip-on-state-id branch June 9, 2023 15:53
@jmhooper jmhooper mentioned this pull request Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants